Fix TypeError when JSON login password is a non-string value - #1256
Fix TypeError when JSON login password is a non-string value#1256miettal wants to merge 1 commit into
Conversation
LoginForm.validate() only checks password.data truthiness via DataRequired()/InputRequired(), not its type. A JSON login body can set password to a dict/list/number/bool instead of a string, which then crashes hash_password()/get_hmac() or password_util.normalize() with an unhandled TypeError instead of failing validation cleanly. Add an isinstance check right after the existing password-required assertion so non-string passwords are rejected like any other invalid login.
|
thanks for pointing this out. wow. This is a bigger issue than password - and I think the solution should involve all the StringField() inputs - possibly with a custom validator. |
|
Thank you for the quick response. |
|
Ok - always looking for contributors - this will be a fairly extensive change and I would want to get this into the upcoming 5.9 release (hopefully in the next month). As part of pallets-eco we strive for human collaborators and will quickly close out obvious AI generated PRs. I think the right approach is to create a validator in forms.py: `class IsString(ValidatorMixin): def call: ` Then add this to most/all of the form StringField. Note that there are a few files that define forms that also have StringField. Would also want to add the validator to EmailFields.... Testing would include at least one test that the message is getting properly localized. Whew! Let me know if you are up for tackling this... |
|
OK!, I'll write code by hand mainly and Use AI is limited to assistance. |
…values from JSON requests (#1258) Replace #1256 with a root-cause fix. - add IsString() validator. - add IsStringOrInt() validator. - add IsString/IsStringOrInt validation for each string-required field. - add a test case confirming translation works. - add a test case confirming the new validators work. - add special handling for ConfirmRegisterForm/RegisterFormV2. - add a test case for special handling for ConfirmRegisterForm/RegisterFormV2.
|
Another solution was merged, This this PR is close. |
2 participants
Summary
LoginForm.validate()only checks ifpassword.datais truthy, not its type. Flask-WTF supports JSON request bodies, so a login POST with a non-stringpassword(e.g. a dict) passes this check.That value then reaches
hash_password()/get_hmac()orpassword_util.normalize()unchanged, and both crash with an unhandledTypeErrorinstead of a normal validation error.Fix
Add an
isinstance(self.password.data, str)check inLoginForm.validate(), right after the existing password-required assertion. Non-string passwords are now rejected like any other invalid login.Testing
test_non_string_json_password_authintests/test_basic.py.main(reverted the fix locally) and passes with the fix.test_basic.pysuite locally: no new failures (a few pre-existing skips for optional deps I don't have installed: pymongo, peewee, asgiref, flask_sqlalchemy_lite).flake8andblack --checkare clean.Found via a Sentry error recurring across several apps that all use Flask-Security-Too's default JSON login, so this likely affects any app accepting JSON logins.